-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[lldb-dap] Add stdio redirection #158609
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[lldb-dap] Add stdio redirection #158609
Conversation
@llvm/pr-subscribers-lldb Author: Druzhkov Sergei (DrSergei) ChangesAs far as I understand, lldb-dap does not currently support stdio redirection. I have added support for this via a new field in the launch configuration named Full diff: https://github.com/llvm/llvm-project/pull/158609.diff 7 Files Affected:
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index 9fe8ca22e820b..daa3e76df6d82 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -1039,6 +1039,7 @@ def request_launch(
disableSTDIO=False,
shellExpandArguments=False,
console: Optional[str] = None,
+ stdio: Optional[list[str]] = None,
enableAutoVariableSummaries=False,
displayExtendedBacktrace=False,
enableSyntheticChildDebugging=False,
@@ -1090,6 +1091,8 @@ def request_launch(
args_dict["sourceMap"] = sourceMap
if console:
args_dict["console"] = console
+ if stdio:
+ args_dict["stdio"] = stdio
if postRunCommands:
args_dict["postRunCommands"] = postRunCommands
if customFrameFormat:
diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
index 22fcd42b3d36a..a301e00f6755f 100644
--- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
+++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
@@ -6,6 +6,7 @@
from lldbsuite.test.lldbtest import *
import lldbdap_testcase
import os
+import pathlib
import re
import tempfile
@@ -624,3 +625,23 @@ def test_no_lldbinit_flag(self):
# Verify the initCommands were executed
self.verify_commands("initCommands", output, initCommands)
+
+ def test_stdio_redirection(self):
+ """
+ Test stdio redirection.
+ """
+ temp_file = tempfile.NamedTemporaryFile().name
+ self.build_and_create_debug_adapter()
+ program = self.getBuildArtifact("a.out")
+
+ self.launch(program, stdio=[None, temp_file, None])
+ self.continue_to_exit()
+
+ try:
+ with open(temp_file, "r") as f:
+ lines = f.readlines()
+ self.assertIn(
+ program, lines[0], "make sure program path is in first argument"
+ )
+ finally:
+ pathlib.Path(temp_file).unlink(missing_ok=True)
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
index 4fadf1c22e0e3..4b727e1e33a64 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
@@ -177,6 +177,31 @@ llvm::Error BaseRequestHandler::LaunchProcess(
launch_info.SetEnvironment(env, true);
}
+ if (!arguments.stdio.empty() && !arguments.disableSTDIO) {
+ size_t n = std::max(arguments.stdio.size(), static_cast<size_t>(3));
+ for (size_t i = 0; i < n; i++) {
+ std::optional<std::string> path;
+ if (arguments.stdio.size() < i)
+ path = arguments.stdio.back();
+ else
+ path = arguments.stdio[i];
+ if (!path)
+ continue;
+ switch (i) {
+ case 0:
+ launch_info.AddOpenFileAction(i, path->c_str(), true, false);
+ break;
+ case 1:
+ case 2:
+ launch_info.AddOpenFileAction(i, path->c_str(), false, true);
+ break;
+ default:
+ launch_info.AddOpenFileAction(i, path->c_str(), true, true);
+ break;
+ }
+ }
+ }
+
launch_info.SetDetachOnError(arguments.detachOnError);
launch_info.SetShellExpandArguments(arguments.shellExpandArguments);
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
index e1806d6230a80..b455112cd37d9 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -303,7 +303,8 @@ bool fromJSON(const json::Value &Params, LaunchRequestArguments &LRA,
O.mapOptional("disableSTDIO", LRA.disableSTDIO) &&
O.mapOptional("shellExpandArguments", LRA.shellExpandArguments) &&
O.mapOptional("runInTerminal", LRA.console) &&
- O.mapOptional("console", LRA.console) && parseEnv(Params, LRA.env, P);
+ O.mapOptional("console", LRA.console) &&
+ O.mapOptional("stdio", LRA.stdio) && parseEnv(Params, LRA.env, P);
}
bool fromJSON(const json::Value &Params, AttachRequestArguments &ARA,
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index 0848ee53b4410..92dada2295841 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -300,6 +300,8 @@ struct LaunchRequestArguments {
/// terminal or external terminal.
Console console = eConsoleInternal;
+ std::vector<std::optional<std::string>> stdio;
+
/// @}
};
bool fromJSON(const llvm::json::Value &, LaunchRequestArguments &,
diff --git a/lldb/tools/lldb-dap/README.md b/lldb/tools/lldb-dap/README.md
index 39dabcc1342c8..52bb009b9c796 100644
--- a/lldb/tools/lldb-dap/README.md
+++ b/lldb/tools/lldb-dap/README.md
@@ -237,6 +237,7 @@ contain the following key/value pairs:
| **stopOnEntry** | boolean | | Whether to stop program immediately after launching.
| **runInTerminal** (deprecated) | boolean | | Launch the program inside an integrated terminal in the IDE. Useful for debugging interactive command line programs.
| **console** | string | | Specify where to launch the program: internal console (`internalConsole`), integrated terminal (`integratedTerminal`) or external terminal (`externalTerminal`). Supported from lldb-dap 21.0 version.
+| **stdio** | [string] | | Destination for program stdio streams (0 - stdin, 1 - stdout, 2 - stderr, ...). Using `null` value means no redirection. Supported from lldb-dap 22.0 version.
| **launchCommands** | [string] | | LLDB commands executed to launch the program.
For JSON configurations of `"type": "attach"`, the JSON configuration can contain
diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json
index 0290a5f18f800..631b768b37c80 100644
--- a/lldb/tools/lldb-dap/package.json
+++ b/lldb/tools/lldb-dap/package.json
@@ -615,6 +615,14 @@
"description": "Specify where to launch the program: internal console, integrated terminal or external terminal.",
"default": "internalConsole"
},
+ "stdio": {
+ "type": "array",
+ "items": {
+ "type": "string"
+ },
+ "description": "Destination for program stdio streams (0 - stdin, 1 - stdout, 2 - stderr, ...). Using null value means no redirection.",
+ "default": []
+ },
"timeout": {
"type": "number",
"description": "The time in seconds to wait for a program to stop at entry point when launching with \"launchCommands\". Defaults to 30 seconds."
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this!
""" | ||
Test stdio redirection. | ||
""" | ||
temp_file = tempfile.NamedTemporaryFile().name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use this pattern
with tempfile.TemporaryFile() as fp:
...
that way you don't need to unlink the file manually
launch_info.SetEnvironment(env, true); | ||
} | ||
|
||
if (!arguments.stdio.empty() && !arguments.disableSTDIO) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this to a helper function because this function is already very big
lldb/tools/lldb-dap/README.md
Outdated
| **stopOnEntry** | boolean | | Whether to stop program immediately after launching. | ||
| **runInTerminal** (deprecated) | boolean | | Launch the program inside an integrated terminal in the IDE. Useful for debugging interactive command line programs. | ||
| **console** | string | | Specify where to launch the program: internal console (`internalConsole`), integrated terminal (`integratedTerminal`) or external terminal (`externalTerminal`). Supported from lldb-dap 21.0 version. | ||
| **stdio** | [string] | | Destination for program stdio streams (0 - stdin, 1 - stdout, 2 - stderr, ...). Using `null` value means no redirection. Supported from lldb-dap 22.0 version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you pretty much include all the contents from here
The stdio property is a list of redirection targets for each of the debuggee's stdio streams:
null value will cause redirect to the default debug session terminal (as specified by the terminal launch property),
"/some/path" will cause the stream to be redirected to the specified file, pipe or a TTY device*,
if you provide less than 3 values, the list will be padded to 3 entries using the last provided value,
you may specify more than three values, in which case additional file descriptors will be created (4, 5, etc.)
Examples:
"stdio": [null, "log.txt", null] - connect stdin and stderr to the default terminal, while sending stdout to "log.txt",
"stdio": ["input.txt", "log.txt"] - connect stdin to "input.txt", while sending both stdout and stderr to "log.txt",
"stdio": null - connect all three streams to the default terminal.
? That will be very clear
lldb/tools/lldb-dap/package.json
Outdated
"items": { | ||
"type": "string" | ||
}, | ||
"description": "Destination for program stdio streams (0 - stdin, 1 - stdout, 2 - stderr, ...). Using null value means no redirection.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here about
The stdio property is a list of redirection targets for each of the debuggee's stdio streams:
null value will cause redirect to the default debug session terminal (as specified by the terminal launch property),
"/some/path" will cause the stream to be redirected to the specified file, pipe or a TTY device*,
if you provide less than 3 values, the list will be padded to 3 entries using the last provided value,
you may specify more than three values, in which case additional file descriptors will be created (4, 5, etc.)
Examples:
"stdio": [null, "log.txt", null] - connect stdin and stderr to the default terminal, while sending stdout to "log.txt",
"stdio": ["input.txt", "log.txt"] - connect stdin to "input.txt", while sending both stdout and stderr to "log.txt",
"stdio": null - connect all three streams to the default terminal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
As far as I understand, lldb-dap does not currently support stdio redirection. I have added support for this via a new field in the launch configuration named
stdio
. It was inspired by the same named field in CodeLLDB.